-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Migrate from Yarn to pnpm #46
Conversation
[pnpm](https://github.com/pnpm/pnpm) is a package manager optimized for monorepos. It is faster than NPM and consumes less disk storage than Yarn. Disk consumption: - Yarn ```sh $ du -sh ~/workspace/acre/acre 2.2G /Users/jakub/workspace/acre/acre ``` - pnpM ```sh $ du -sh ~/workspace/acre/acre 875M /Users/jakub/workspace/acre/acre ``` To install pnpm with Homebrew run `brew install pnpm`, for other installation options please see the [documentation](https://pnpm.io/installation). To install the packages dependencies run: ```sh pnpm install ```
Use pnpm instead of yarn in CI workflow. Define `packageManager` in `package.json` so the `pnpm/action-setup@v2` job uses it to setup the correct pnpm version.
Gatsby requires a plugin to work with pnpm.
Be explicit in the command that we want to run a script and pass the paratemeters to the script
Instead of adding dependencies we define public-hoist-pattern property for @chakra-ui/* packages. Read more on public-hoist-pattern here: https://pnpm.io/npmrc#public-hoist-pattern The fix was suggested in chakra-ui/chakra-ui#6427 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we get rid of |
Define main .gitignore in the root directory with a common rules and separate .gitignore files with module-specific rules.
dApp and website docs were missing pnpm commands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's resolve conflicts. Other than that LGTM 🚀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some comments:
- I wonder what is the correct approach for
pnpm
. Should the packages for each repository be installed separately? From the main app root, we can install all packages using thepnpm install
. In the case of the website, everything works correctly when packages are installed from the root. For dApp there is a problem with@chakra-ui/theme-tools
. I would like to clear up any doubts about the way packages are installed.
- If it is possible to install packages from root level then I would suggest adding the
.nvmrc
file.
- This thread has already been discussed here. I wonder if we need the
.nmprc
file.
The root directory contains package.json, so it is worth adding the .nvmrc file for consistency.
When npmrc was defined with public hoist in the `dapp` directory it required installation of the dependencies to happen inside the `dapp` directory to correctly resolve the `@chakra-ui/theme-tools` dependency. With moving the file to the root directory, installation of the dependencies inside the root directory will correctly resolve `@chakra-ui` dependencies inside the `dapp` sub-project.
@kkosiorowska thank you for the comments!
Fixed in 08a3668
Added in d34b104
Please check with the latest changes. It worked fine on my machine: *[pnpm][~/workspace/acre/acre/dapp]$ pnpm lint:js
> [email protected] lint:js /Users/jakub/workspace/acre/acre/dapp
> eslint .
=============
WARNING: You are currently running a version of TypeScript which is not officially supported by @typescript-eslint/typescript-estree.
You may find that it works just fine, or you may not.
SUPPORTED TYPESCRIPT VERSIONS: >=4.3.5 <5.3.0
YOUR TYPESCRIPT VERSION: 5.3.2
Please only submit bug reports when using the officially supported version.
============= |
The job has to be updated to use pnpm instead of yarn.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All comments have been implemented. Ready for merge 🚀
We need .npmrc
to properly install the dependencies. However, this change doesn't solve the problem with import/no-extraneous-dependencies
. Let's leave the rule as it is for now.
Depends on #46 ### Syncpack [Syncpack](https://jamiemason.github.io/syncpack/) is a tool that helps manage multiple package.json files in a monorepo. #### Install In the repository's root directory execute: ```sh pnpm install ``` #### Usage To list dependencies from all packages run: ```sh pnpm syncpack list ``` To update a dependency (e.g. `eslint`) in all packages run: ```sh pnpm syncpack update --filter eslint ``` ### Example ![image](https://github.com/thesis/acre/assets/10741774/a948865e-c10d-464e-822d-f9d734d1dfb8)
Use pnpm as a package manager
pnpm is a package manager optimized for monorepos.
It is faster than NPM and consumes less disk storage than Yarn.
Disk consumption:
$ du -sh ~/workspace/acre/acre 2.2G /Users/jakub/workspace/acre/acre
$ du -sh ~/workspace/acre/acre 875M /Users/jakub/workspace/acre/acre
To install pnpm with Homebrew run
brew install pnpm
, for other installation optionsplease see the documentation.
To install the packages dependencies run: